Support CPU profiling sections of code #3971

breezewish · 2018-12-24T08:55:46Z

Signed-off-by: Breezewish breezewish@pingcap.com

What have you changed? (mandatory)

We usually need to find out why specific sections of code are slow. However, sometimes it cannot be done via simple perf, i.e. when there is a bootstrap. This PR provides facility to programmatically start or stop CPU profiling. In this way, we can start profiling before our interested code begins and stop profiling after it ends.

We can explore more interesting usages in the future, i.e. toggle profiling dynamically via signals, or environment variables. Future PRs are welcome!

The manual is in the code. I paste it here for easy reading:

Profile a part of the code using CPU Profiler from gperftools or Callgrind.

Requirements

Linux

Other OS may also work however, not tested.
gperftools

You can follow its INSTALL manual.
Roughly the instructions are the following:
1. Download packages from release
2. Run ./configure
3. Run make install

Usage

profiler::start("./app.profile");
some_complex_code();
profiler::stop();

Then, compile the code with profiling feature enabled and run the code with environment
variable TIKV_PROFILE=1.

By default, a profile called app.profile will be generated by CPU Profiler.
You can then analyze the profile using pprof.

If the application is running in Callgrind, a Callgrind profile dump will be generated instead.
Notice that you should run Callgrind with command line option --instr-atstart=no, e.g.:

TIKV_PROFILE=1 valgrind --tool=callgrind --instr-atstart=no ./my_example

Also see examples/prime.rs.

Signed-off-by: Breezewish <breezewish@pingcap.com>

siddontang · 2018-12-24T13:44:29Z

Do we need to install gperftools at first?

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish · 2018-12-24T16:13:36Z

@siddontang Yes. I copied the manual from code to the PR description now. I also updated the code to support Callgrind (so that we can profile cache hit etc).

Signed-off-by: Breezewish <breezewish@pingcap.com>

siddontang · 2018-12-25T07:43:16Z

So if the user doesn't install perftools, can it build TiKV now?

I suggest you introduce valgrind in another PR.

breezewish · 2018-12-25T14:36:19Z

@siddontang Yes. Only when profiling feature is enabled, gperftools is required. As you can see, it builds normally on Circle CI and our own Jenkins CI.

The support of Valgrind is only 4 lines of code. It should not be a big problem.

siddontang · 2018-12-26T00:47:30Z

do we need to install vagrind manally too?

siddontang · 2018-12-26T00:50:39Z

IMO, if we build with this feature, we don't need using environment forcibly to control it. Only calling start and stop is enough.

breezewish · 2018-12-26T04:26:14Z

@siddontang Don't need to install valgrind to build it.

Ok

breezewish · 2019-01-02T13:31:59Z

Updated. No need to set "TIKV_PROFILE=1" now.

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish · 2019-04-10T16:56:51Z

@brson PTAL for this~ Thanks a lot!

Signed-off-by: Breezewish <breezewish@pingcap.com>

brson · 2019-04-12T02:28:22Z

This is very cool @breeswish.

I guess that the idea is to insert start and stop temporarily and then remove them when done? Otherwise there's a lot of deadlock potential.

One nice change would be to change the profiler manifest to make all the dependencies optional (particularly the profiling deps), and have the profiling target enable them. That would save the download, build, and link of the profiler crates.

Is there anything holding up landing this?

Maybe docs - like the allocator configuration this seems like something that should be surfaced in a developer's guide. Not necessary for this PR though.

components/profiler/Cargo.toml

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish · 2019-04-12T09:17:08Z

@brson @kennytm Thanks a lot for reviewing! I have updated my Cargo.toml to make these dependencies optional (not sure it there are better ways). Also updated docs for MacOS instructions.

I guess that the idea is to insert start and stop temporarily and then remove them when done? Otherwise there's a lot of deadlock potential.

Yes. A better way maybe, provide interfaces to trigger a start and stop it moment later so that a profile can be generated. This does not introduce runtime costs when profiling is not started. The interface can be our status server like #4444

Is there anything holding up landing this?

Nop, it's just lack of reviewers previously :)

siddontang · 2019-04-14T11:43:50Z

@brson

I think we can advance this PR now.
But here I prefer using HTTP API to control profile.

breezewish · 2019-04-14T12:05:04Z

@siddontang We can have another PR that utilize the interface via HTTP API.

siddontang · 2019-04-25T13:12:25Z

PTAL @brson

brson · 2019-04-26T03:13:23Z

@siddontang acknowledged that we want an HTTP interface, and that this is a good start.

brson · 2019-04-26T03:14:06Z

lgtm

BusyJay

Can gperftools be installed automatically?

BusyJay · 2019-04-26T03:23:38Z

components/profiler/Cargo.toml

+publish = false
+
+[features]
+profiling = ["lazy_static", "cpuprofiler", "callgrind", "valgrind_request"]


Can it compile on Windows without the feature enabled?

It should work though I don't have a Windows machine. I changed it to [target.'cfg(linux)'.dependencies] and it can compile on my MacOS.

BusyJay · 2019-04-26T03:32:38Z

components/profiler/src/lib.rs

+//! Also see `examples/prime.rs`.
+
+#[allow(unused_extern_crates)]
+extern crate tikv_alloc;


Why use tikv_alloc?

tikv_alloc generally needs to be linked into every crate that doesn't link to tikv, so that tests and benches of that crate use tikv's allocator. I don't think this extern crate statement is needed though as long as the dependency exists.

CI will fail if alloc is not linked

Ah, right. There are tests that all binaries contain jemalloc.

components/profiler/src/lib.rs

components/profiler/src/profiler_linux.rs

BusyJay · 2019-04-26T03:41:36Z

components/profiler/Cargo.toml

+[target.'cfg(unix)'.dependencies]
+lazy_static = { version = "1.2.0", optional = true }
+cpuprofiler = { version = "0.0.3", optional = true }
+callgrind = { version = "1.1.0", optional = true }


Is valgrind a useful use case? I think most of the time only cpuprofiling is used.

valgrind is very useful for micro benchmark, which can report precise amount of function calls, as well as precise (emulated) cache hit.

I'm also hoping that this crate can be extended into a general purpose profiling module that can be published for the community, and ultimately fulfill the various requirements of the Go profiling tools. More profiler options in that case seems good.

Signed-off-by: Breezewish <breezewish@pingcap.com>

siddontang

LGTM

brson

LGTM

Signed-off-by: Breezewish <breezewish@pingcap.com>

Support profiling sections of code

b156a05

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish force-pushed the __profiler branch from 28285d6 to 5d8d429 Compare December 24, 2018 16:10

Support Callgrind

9ddf5e9

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish force-pushed the __profiler branch from 5d8d429 to 9ddf5e9 Compare December 24, 2018 16:11

tikv deleted a comment from zhouqiang-cl Dec 24, 2018

tikv deleted a comment from siddontang Dec 24, 2018

breezewish added 2 commits December 25, 2018 00:17

Make rustfmt and clippy happy

2091eda

Signed-off-by: Breezewish <breezewish@pingcap.com>

Fix doc test

35eb12c

Signed-off-by: Breezewish <breezewish@pingcap.com>

This comment has been minimized.

Sign in to view

Connor1996 added the component/performance Component: Performance label Dec 25, 2018

breezewish added 2 commits January 2, 2019 21:32

start profiling without env variables

213229e

Signed-off-by: Breezewish <breezewish@pingcap.com>

Merge remote-tracking branch 'origin/master' into __profiler

1827120

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish force-pushed the __profiler branch from b660fc4 to 1827120 Compare January 2, 2019 13:33

Fix lock

05f25ac

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish mentioned this pull request Apr 7, 2019

Link tcmalloc statically #4471

Closed

Merge remote-tracking branch 'origin/master' into __profiler

6003a20

Signed-off-by: Breezewish <breezewish@pingcap.com>

kennytm reviewed Apr 12, 2019

View reviewed changes

components/profiler/Cargo.toml Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into __profiler

27d4973

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish requested a review from brson April 12, 2019 09:17

breezewish and others added 5 commits April 15, 2019 12:41

Merge branch 'master' into __profiler

0c7e44b

Merge branch 'master' into __profiler

8105a60

Merge branch 'master' into __profiler

d7e919f

Merge branch 'master' into __profiler

9c677dd

Merge branch 'master' into __profiler

f4c67e5

Merge branch 'master' into __profiler

3998965

brson previously approved these changes Apr 26, 2019

View reviewed changes

brson added the status/LGT1 Status: PR - There is already 1 approval label Apr 26, 2019

BusyJay reviewed Apr 26, 2019

View reviewed changes

breezewish added 3 commits April 29, 2019 00:33

Address comments about the returning value

5bc7552

Signed-off-by: Breezewish <breezewish@pingcap.com>

Merge remote-tracking branch 'origin/master' into __profiler

ed94239

Upgrade dependency

e22fe77

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish dismissed brson’s stale review via e22fe77 April 28, 2019 16:41

breezewish and others added 3 commits April 29, 2019 11:27

Merge branch 'master' into __profiler

3fe352c

Merge branch 'master' into __profiler

dd57f1d

Merge branch 'master' into __profiler

0df8cc7

siddontang approved these changes May 6, 2019

View reviewed changes

Merge branch 'master' into __profiler

8db777f

brson approved these changes May 6, 2019

View reviewed changes

Merge branch 'master' into __profiler

048008c

breezewish merged commit 68802bb into tikv:master May 6, 2019

breezewish deleted the __profiler branch May 6, 2019 13:57

jswh pushed a commit to jswh/tikv that referenced this pull request May 27, 2019

Support CPU profiling sections of code (tikv#3971)

daaf108

Signed-off-by: Breezewish <breezewish@pingcap.com>

breezewish mentioned this pull request Jul 23, 2019

Support generating flame graph from HTTP API #5124

Closed

sticnarf pushed a commit to sticnarf/tikv that referenced this pull request Oct 27, 2019

Support CPU profiling sections of code (tikv#3971)

447ff27

Signed-off-by: Breezewish <breezewish@pingcap.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support CPU profiling sections of code #3971

Support CPU profiling sections of code #3971

breezewish commented Dec 24, 2018 •

edited

siddontang commented Dec 24, 2018

breezewish commented Dec 24, 2018

This comment has been minimized.

siddontang commented Dec 25, 2018

breezewish commented Dec 25, 2018 •

edited

siddontang commented Dec 26, 2018

siddontang commented Dec 26, 2018

breezewish commented Dec 26, 2018

breezewish commented Jan 2, 2019

breezewish commented Apr 10, 2019

brson commented Apr 12, 2019

breezewish commented Apr 12, 2019 •

edited

siddontang commented Apr 14, 2019

breezewish commented Apr 14, 2019

siddontang commented Apr 25, 2019

brson commented Apr 26, 2019

brson commented Apr 26, 2019

BusyJay left a comment

BusyJay Apr 26, 2019

breezewish Apr 28, 2019

BusyJay Apr 26, 2019

brson Apr 26, 2019

breezewish Apr 26, 2019

brson Apr 28, 2019

BusyJay Apr 26, 2019

breezewish Apr 26, 2019

brson Apr 28, 2019

siddontang left a comment

brson left a comment

Support CPU profiling sections of code #3971

Support CPU profiling sections of code #3971

Conversation

breezewish commented Dec 24, 2018 • edited

What have you changed? (mandatory)

Requirements

Usage

siddontang commented Dec 24, 2018

breezewish commented Dec 24, 2018

This comment has been minimized.

siddontang commented Dec 25, 2018

breezewish commented Dec 25, 2018 • edited

siddontang commented Dec 26, 2018

siddontang commented Dec 26, 2018

breezewish commented Dec 26, 2018

breezewish commented Jan 2, 2019

breezewish commented Apr 10, 2019

brson commented Apr 12, 2019

breezewish commented Apr 12, 2019 • edited

siddontang commented Apr 14, 2019

breezewish commented Apr 14, 2019

siddontang commented Apr 25, 2019

brson commented Apr 26, 2019

brson commented Apr 26, 2019

BusyJay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siddontang left a comment

Choose a reason for hiding this comment

brson left a comment

Choose a reason for hiding this comment

breezewish commented Dec 24, 2018 •

edited

breezewish commented Dec 25, 2018 •

edited

breezewish commented Apr 12, 2019 •

edited